Add useArgumentFile option to help with Command line is too long error #42
Conversation
cheister
force-pushed
the
useArgumentFileOption
branch
from
66e2824
to
1ca7f92
Compare
|
Thanks for that, I think that should be good enough for everyone. I am going to update the documentation and FAQ. |
cheister
force-pushed
the
useArgumentFileOption
branch
from
1ca7f92
to
dc0bcdd
Compare
|
SG. I added one basic integration test for the useArgumentFile option. |
|
Don’t worry, I have created one last night, I only need to retest the whole thing on Linux.
Thank you for your contribution!
Kind regards,
Sergey Ivanov
On 10 May 2018, at 07:56, cheister <[email protected]> wrote:
SG. I add one basic integration test for the useArgumentFile option.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub, or mute the thread.
|
- added an integration test for useArgumentFile option - fixed writing unicode file names to the argument file - fixed handling of unicode output from protoc - simplified path normalisation, tested on both Windows and Linux
|
Hi, I've pushed my changes to your PR branch. I've tested them on both Windows and Linux, and the tests pass. I ended up removing path normalisation, because everything works without it on both OSes. I have also added better handling of unicode output and unicode file names (covered by the new integration test too). Can you please test it on MacOS by running: If that works, we are good. I'll merge it as soon as you let me know that you're happy with the changes. Sergey |
|
The changes look good to me. The above command also works on OSX (10.13) for me. |
This is primarily to address #5
I've only tested it on OSX and Linux. I don't have a Windows setup to test with.
A nicer option might be to detect if protoc is 3.5.0 or higher and then automatically put the args in the argumentFile but I wasn't sure how to get the protoc version in a nice way.